I think there is a certain issue with the current patch. Functions exported by C++/WinRT have extern "C++", which allows them to link with header files and share the same symbols. If you remove extern "C++" from get_marshaler (produced by WINRT_EXPORT), the get_marshaler inside the module will generate its own symbol. As a result, functions exported by the C++/WinRT module will have the same symbols as the header file users, but refer to different symbols of get_marshaler, leading to an ODR violation. I haven't looked into other workarounds in detail yet, but as I mentioned earlier, we might consider separating extern "C++" from WINRT_EXPORT and moving it to the .ixx file, like:

extern "C++" {
#include "winrt/Windows.Foundation.h"
}

Also, keep get_marshaler from being exported.

I don't think it's an ODR violation. The module's exported symbols will have module linkage, meaning they are not the same symbols. The only place we really need to use extern "C++" is when symbols need to be shared, like the winrt_get_activation_handler and other function pointer interfaces.

No, extern "C++" and module linkage are unrelated things. The STL wraps everything in std with extern "C++", which is consistent with the current C++/WinRT approach, and that was intentional on my part.

Note the comment

// In the STL's headers (which might be used to build the named module std), we unconditionally
// and directly mark declarations of our separately compiled machinery as extern "C++", allowing
// the named module to work with the separately compiled code (which is always built classically).

// TRANSITION: _USE_EXTERN_CXX_EVERYWHERE_FOR_STL controls whether we also wrap the STL's
// header-only code in this linkage-specification, as a temporary workaround to allow
// importing the named module in a translation unit with classic includes.

This should not be a problem for us as we don't really support mixing includes and imports. We also do not have much in terms of separately built machinery.

This should not be a problem for us as we don't really support mixing includes and imports.

In my own fork, I have already implemented support for mixing includes and imports, and C++/WinRT has inherited this as well.

TL;DR

I'm not worried about ODR violations on get_marshaler. It's not exported, it lives in a single module, winrt_base, and conflicting definitions should already be protected by the existing C++/WinRT version checks. However, we should test if the lack of extern "C++" in this PR breaks include-then-import.

Wall of text

Linkage and extern "C++" are related concepts, as I understand it.

By default, declarations in a module after export module FOO have module linkage and are owned by the module. If multiple declarations of an entity exist in a program attached to different modules, the program is ill-formed. I'm not aware if the standard mandates behavior for a module-owned declaration coexisting with a declaration not attached to a module (mixing include and import), so I don't know which toolchains are standards-compliant. But MSVC rejects them in most cases. I suspect Clang is more standard-accurate in at least some cases, given other experiments I've performed.

extern "C++" specifies language linkage, and putting it on a declaration in a module detaches it from that module "ownership", allowing it to coexist in multiple modules, and in MSVC at least, allowing the module declaration to coexist in non-module (aka header) declarations.

Any hope of adopting modules in real world projects will require headers and modules to coexist to some degree. There are many large projects already using C++/WinRT that will need to migrate incrementally. There are also external libraries and tools with widespread usage that assume C++/WinRT is in header form, or outright generate #include <winrt/Foo.h> code today.

We can get MSVC to accept include-then-import by detaching these declarations from module ownership and module linkage by giving them language linkage instead, aka extern "C++". This allows declarations to tolerate include-then-import. Declarations that import-then-include is still broken, and almost certainly requires compiler fixes before it will work. This is why C++/WinRT has WINRT_IMPORT_MODULE - sometimes you have pesky #include <winrt/Foo.h> directives that you can't easily delete (see external party libs/tools) - with this macro, you can #include the winrt headers, but they won't introduce any new declarations that could conflict with what was already brought in via modules.

This, along with the separately compiled machinery, is what I believe to be the primary motivators for the STL to use language linkage by saying extern "C++".

But extern "C++" has another benefit beyond header-module coexistence. In #1588, I'm proposing a significant improvement in compilation cost by having cppwinrt.exe precalculate certain parameterized interface (aka pinterface) IIDs, by specializing winrt::impl::pinterface_guid<T>. The problem is, which module should own a given specialization? Take the actual API, IAsyncOperation<StorageFile> FileLoggingSession.CloseAndSaveToFileAsync() in the Windows.Foundation.Diagnostics namespace. The type responsible for this generic type instantiation, FileLoggingSession lives in winrt.Windows.Foundation.Diagnostics, IAsyncOperation<T> lives in winrt.Windows.Foundation, and StorageFile lives in winrt.Windows.Storage. And this says nothing of the many other APIs that could also return IAsyncOperation<StorageFile>. Attempting to decide a canonical owner of the specialization is not going to work, especially when you need to coexist with component authors that can introduce new APIs of their own. So, the answer is to detach these winrt::impl::pinterface_guid specializations from any C++/WinRT module by giving them language linkage instead.

The STL is not a code generator. It has a single set of headers and module interface files it ships, and has no interest in sharing its declarations with other modules. C++/WinRT, on the other hand, generates headers and module interfaces for whatever WinRT APIs it is fed, from multiple sources (SDK, Nuget/Project references, IDL/winmd) and so we need to allow for a certain amount of shared ownership. This was true before modules, which is part of why C++/WinRT has long had the version checks to ensure that the mixing and matching is compatible, via a combination of static_assert and #pragma detect_mismatch.